Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle xjc XmlEnumValue in DynamicJAXB enum #2273

Merged
merged 2 commits into from
Nov 14, 2024

Conversation

timtatt
Copy link
Contributor

@timtatt timtatt commented Sep 26, 2024

To resolve #2272

  • Changes EnumInfo literals from List to Map to store intended value
  • Adds @XmlEnumValue annotation to DynamicJAXB enum class
  • Check for @XmlEnumValue when adding enum conversion values

import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please don't use * in imports. We prefer specific imports like removed part.

Copy link
Contributor

@rfelcman rfelcman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In general please use specific types instead of var.

import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.*;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use specific imports instead of ... .* This should be applied to all changes.

@@ -223,7 +223,7 @@ protected void addMethods(ClassWriter cw, String parentClassType) {

protected byte[] createEnum(EnumInfo enumInfo) {

String[] enumValues = enumInfo.getLiteralLabels();
var enumValues = enumInfo.getEnumValues();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use specific type instead of var.

for (String enumValue : enumValues) {
cw.visitField(Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL + Opcodes.ACC_STATIC + Opcodes.ACC_ENUM, enumValue, "L" + internalClassName + ";", null, null);
for (var enumValue : enumValues.entrySet()) {
var fv = cw.visitField(Opcodes.ACC_PUBLIC + Opcodes.ACC_FINAL + Opcodes.ACC_STATIC + Opcodes.ACC_ENUM, enumValue.getKey(), "L" + internalClassName + ";", null, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

var again.

@@ -92,6 +95,16 @@ public void initialize(DatabaseMapping mapping, Session session) {
super.initialize(mapping, session);
}

private String getEnumValue(Enum theEnum) {
try {
Field field = theEnum.getClass().getField(theEnum.name());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use (maybe add new method into) https://github.com/eclipse-ee4j/eclipselink/blob/master/moxy/org.eclipse.persistence.moxy/src/main/java/org/eclipse/persistence/jaxb/ReflectionUtils.java
to ensure, that PrivilegedAccessHelper.callDoPrivilegedWithException( is used instead of direct call.

Copy link
Contributor

@rfelcman rfelcman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rfelcman rfelcman merged commit ac4dc50 into eclipse-ee4j:master Nov 14, 2024
6 checks passed
@rfelcman
Copy link
Contributor

It was merged into master -> upcoming 5.x version. If You want it into EL 4.x please make a backport against 4.0 branch.

@timtatt
Copy link
Contributor Author

timtatt commented Nov 18, 2024

@rfelcman thanks for reviewing. Is it possible to kick off a beta promotion to publish to mvnrepository?

@rfelcman
Copy link
Contributor

There is snapshot repo
https://jakarta.oss.sonatype.org/content/repositories/staging/org/eclipse/persistence/eclipselink/5.0.0-SNAPSHOT/
Every new snapshot is build after PR merge (nightly).
5.0.0-B04 is planned for a tomorrow,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DynamicJAXB doesn't handle @XmlEnumValue() from XJC class
2 participants